Code Review

Bug — memCalcInstallDir directory is never created

In supply_test.go, the MkdirAll call for the memory calculator install directory uses the wrong variable:

// create memory calculator install dir
memCalcInstallDir := filepath.Join(depsDir, depsIdx, "tmp", "memory-calculator")
Expect(os.MkdirAll(filepath.Join(jvmkillInstallDir), 0755)).To(Succeed()) // should be memCalcInstallDir

jvmkillInstallDir is used again instead of memCalcInstallDir, so the memory calculator directory is never created on disk. Since mockInstaller mocks out the actual install call, this likely won't cause a test failure today — but it's incorrect and will break if any post-install code ever tries to read from that directory.

Fix: Replace jvmkillInstallDir with memCalcInstallDir in that MkdirAll call.

Minor — Dead Installer interface in cf_metrics_exporter.go

After this PR removes the installer Installer field from CfMetricsExporterFramework, the local Installer interface defined at the top of cf_metrics_exporter.go is no longer referenced anywhere and can be removed.

Low — github.com/golang/mock is archived

github.com/golang/mock v1.6.0 was officially archived in June 2023 and receives no further updates or security fixes. The community-maintained fork go.uber.org/mock has an identical API and would be a better long-term choice. Not a blocker, but worth considering before this becomes harder to migrate.

Observation — Finalize tests are a hybrid integration/unit style

The finalize tests set finalizer.JREName directly and use a real libbuildpack.Stager alongside the mocked manifest/installer. This means jre.Finalize() (called via resolveJRE()) will perform real filesystem writes through the stager. This isn't a bug, but the mixed approach can obscure what's actually being unit tested vs. integration tested.

Observation — No test for the "no recognized application type" error path

Now that supply.Run() is testable via mocks, a test covering the case where no container is detected (→ Run() returns an error) would be straightforward to add and fills a meaningful gap that was previously only noted in a comment pointing to integration tests.